[windows] Fix idempotence when reinstalling same pinned version #269
Conversation
| # the package). | ||
| - name: Check currently installed Agent version | ||
| win_shell: | | ||
| $product_name = "Datadog Agent" |
There was a problem hiding this comment.
On Chef we use agent status for this: https://github.com/DataDog/chef-datadog/blob/483f88a43569b1efd00e7d5c0cb1e479eb09b1a2/libraries/recipe_helpers.rb#L92
I'm not sure which approach is better, but I think we shouldn't have two.
There was a problem hiding this comment.
The Chef approach is slightly better because it would work with rc and beta builds.
However, contrary to this method, it does have a caveat: it only works if you know where the Agent binary is located at (Chef expects it to be C:/Program Files/Datadog/Datadog Agent/bin/agent).
One thing to note is that we currently don't offer the possibility to choose the application directory for Windows install; we do have a datadog_agent_binary_path_windows parameter, but it's not advertised in the README (and thus I suppose it's an internal parameter) and only used for the integration command, so we could use this option here to use agent status (that way people can still configure it for their use case if needed).
There was a problem hiding this comment.
If we have a variable for the Agent path, then I think using that to call the binary is reasonable (assuming we won't change the output of agent status).
I'm not sure this is necessarily better than your approach, though. I would still sync with @kbogtob, who wrote the Chef code I linked before, to discuss pros and cons of both approaches.
What does this PR do?
Fetches the currently installed Agent version to avoid downloading the msi installer whenever possible.
It won't work with rc and beta builds (
Get-WmiObjectdoesn't return these), nor it will work with latest (since we currently have no way of knowing which version "latest" will install), but this should cover most common pinned installs use cases.Motivation
Make role more idempotent.
Should partially fix this issue: #265.